fix: missing parentheses in CREATE POLICY USING clause output#260
Merged
fix: missing parentheses in CREATE POLICY USING clause output#260
Conversation
PostgreSQL's CREATE POLICY syntax requires parentheses around USING and WITH CHECK expressions. When pg_get_expr returns simple function calls without outer parentheses (e.g., is_admin()), pgschema was outputting invalid SQL like `USING is_admin()` instead of `USING (is_admin())`. Added ensureParentheses() helper to wrap expressions in parentheses if not already wrapped. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR ensures that generated CREATE POLICY / ALTER POLICY statements always use the PostgreSQL-required parenthesized syntax for USING and WITH CHECK expressions, even when pg_get_expr omits outer parentheses. It also adds file-based diff fixtures to validate the new behavior against a real-world reproduction of Issue #259.
Changes:
- Updated
internal/diff/policy.goto routeUSINGandWITH CHECKexpressions through a newensureParentheseshelper so that emitted SQL usesUSING (expr)/WITH CHECK (expr)consistently. - Added an
is_admin()helper function and anadmin_onlyRLS policy intestdata/diff/create_policy/add_policyalong with updatedold.sql,new.sql,diff.sql,plan.sql,plan.txt, andplan.jsonfixtures to cover function-call policies and verify correct parentheses in generated SQL. - Refreshed the JSON plan fingerprint and policy entries to include the newly created
admin_onlypolicy.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| testdata/diff/create_policy/add_policy/plan.txt | Adds the admin_only policy to the human-readable diff plan, ensuring the new policy appears in the expected migration steps. |
| testdata/diff/create_policy/add_policy/plan.sql | Updates the expected SQL plan to include CREATE POLICY admin_only ... USING (is_admin());, validating correct parenthesization in generated SQL. |
| testdata/diff/create_policy/add_policy/plan.json | Updates the plan JSON hash and adds a table.policy entry for public.users.admin_only, aligning structured expectations with the new policy. |
| testdata/diff/create_policy/add_policy/old.sql | Introduces the is_admin() function in the “old” schema to set up the pre-diff state for Issue #259. |
| testdata/diff/create_policy/add_policy/new.sql | Adds the admin_only RLS policy using USING (is_admin()); in the “new” schema, providing input DDL for the new regression test. |
| testdata/diff/create_policy/add_policy/diff.sql | Extends the expected migration SQL diff to include the CREATE POLICY admin_only ... USING (is_admin()); statement. |
| internal/diff/policy.go | Wraps policy.Using and policy.WithCheck with ensureParentheses in CREATE/ALTER policy generation and introduces the helper implementation, ensuring compliance with PostgreSQL’s USING (expr) / WITH CHECK (expr) syntax while preserving already-wrapped expressions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PostgreSQL's CREATE POLICY syntax requires parentheses around USING and WITH CHECK expressions. When pg_get_expr returns simple function calls without outer parentheses (e.g., is_admin()), pgschema was outputting invalid SQL like
USING is_admin()instead ofUSING (is_admin()).Added ensureParentheses() helper to wrap expressions in parentheses if not already wrapped.
Fix #259